Skip to content

Conversation

kasuga-fj
Copy link
Contributor

This patch fixes a bug introduced in #145878. A dependency was added in the wrong direction, causing an assertion failure due to broken topological order.

@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2025

@llvm/pr-subscribers-backend-hexagon

Author: Ryotaro Kasuga (kasuga-fj)

Changes

This patch fixes a bug introduced in #145878. A dependency was added in the wrong direction, causing an assertion failure due to broken topological order.


Full diff: https://github.com/llvm/llvm-project/pull/149436.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/MachinePipeliner.cpp (+2-2)
  • (added) llvm/test/CodeGen/Hexagon/swp-load-to-store-forward.mir (+50)
diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp
index b38a4d1c55af9..90005bd181f3a 100644
--- a/llvm/lib/CodeGen/MachinePipeliner.cpp
+++ b/llvm/lib/CodeGen/MachinePipeliner.cpp
@@ -4279,8 +4279,8 @@ void LoopCarriedEdges::modifySUnits(std::vector<SUnit> &SUnits,
             !TII->isGlobalMemoryObject(FromMI) &&
             !TII->isGlobalMemoryObject(ToMI) && !isSuccOrder(From, To)) {
           SDep Pred = Dep;
-          Pred.setSUnit(Src);
-          Dst->addPred(Pred);
+          Pred.setSUnit(From);
+          To->addPred(Pred);
         }
       }
     }
diff --git a/llvm/test/CodeGen/Hexagon/swp-load-to-store-forward.mir b/llvm/test/CodeGen/Hexagon/swp-load-to-store-forward.mir
new file mode 100644
index 0000000000000..2960343564fca
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/swp-load-to-store-forward.mir
@@ -0,0 +1,50 @@
+# RUN: llc -mtriple=hexagon -run-pass pipeliner %s -o /dev/null
+
+# Check that edges that violate topological order are not added to the
+# SwingSchedulerDAG. This is a case where the crash was caused by PR 145878.
+
+--- |
+  target triple = "hexagon"
+  
+  define void @crash_145878() {
+  entry:
+    br label %loop
+  
+  loop:                                             ; preds = %loop, %entry
+    %lsr.iv2 = phi i32 [ %lsr.iv.next, %loop ], [ 1, %entry ]
+    %lsr.iv = phi ptr [ %cgep3, %loop ], [ inttoptr (i32 -8 to ptr), %entry ]
+    %cgep = getelementptr i8, ptr %lsr.iv, i32 12
+    %load = load i32, ptr %cgep, align 4
+    store i32 %load, ptr %lsr.iv, align 4
+    %lsr.iv.next = add nsw i32 %lsr.iv2, -1
+    %iv.cmp.not = icmp eq i32 %lsr.iv.next, 0
+    %cgep3 = getelementptr i8, ptr %lsr.iv, i32 -8
+    br i1 %iv.cmp.not, label %exit, label %loop
+  
+  exit:                                             ; preds = %loop
+    ret void
+  }
+...
+---
+name:            crash_145878
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x80000000)
+  
+    %5:intregs = A2_tfrsi -8
+    J2_loop0i %bb.1, 1, implicit-def $lc0, implicit-def $sa0, implicit-def $usr
+  
+  bb.1.loop (machine-block-address-taken):
+    successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+  
+    %1:intregs = PHI %5, %bb.0, %3, %bb.1
+    %6:intregs = L2_loadri_io %1, 12 :: (load (s32) from %ir.cgep)
+    S2_storeri_io %1, 0, killed %6 :: (store (s32) into %ir.lsr.iv)
+    %3:intregs = A2_addi %1, -8
+    ENDLOOP0 %bb.1, implicit-def $pc, implicit-def $lc0, implicit $sa0, implicit $lc0
+    J2_jump %bb.2, implicit-def dead $pc
+  
+  bb.2.exit:
+    PS_jmpret $r31, implicit-def dead $pc
+...

@kasuga-fj kasuga-fj requested a review from nathanchance July 18, 2025 01:20
@kasuga-fj
Copy link
Contributor Author

@nathanchance Can you verify whether the problem is fixed with this patch?

@nathanchance
Copy link
Member

Yes, this appears to resolve my issue.

@kasuga-fj
Copy link
Contributor Author

Great, thank you for the checks.

@kasuga-fj kasuga-fj requested a review from aankit-ca July 18, 2025 04:08
@androm3da
Copy link
Member

Thanks @nathanchance for the quick diagnosis and @kasuga-fj for the quick fix.

Kasuga-san, would you be willing to cherry pick this to the release/21.x branch?

@kasuga-fj
Copy link
Contributor Author

would you be willing to cherry pick this to the release/21.x branch?

@androm3da Yes, I intended to cherry-pick this after merged to main. Would you prefer that I do it right away?

@androm3da
Copy link
Member

would you be willing to cherry pick this to the release/21.x branch?

@androm3da Yes, I intended to cherry-pick this after merged to main. Would you prefer that I do it right away?

No. In fact that's not permitted IIUC.

See https://llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches for the process

@kasuga-fj
Copy link
Contributor Author

(I completely overlooked the obvious fact that it couldn't be done until after the merge, since we need to specify a commit hash...)

@kasuga-fj kasuga-fj merged commit 6df012a into llvm:main Jul 22, 2025
12 checks passed
@kasuga-fj
Copy link
Contributor Author

/cherry-pick 6df012a

@llvmbot
Copy link
Member

llvmbot commented Jul 22, 2025

/pull-request #149950

@llvmbot llvmbot moved this from Needs Triage to Done in LLVM Release Status Jul 22, 2025
kasuga-fj added a commit to llvmbot/llvm-project that referenced this pull request Jul 24, 2025
This patch fixes a bug introduced in llvm#145878. A dependency was added in
the wrong direction, causing an assertion failure due to broken
topological order.

(cherry picked from commit 6df012a)
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 28, 2025
This patch fixes a bug introduced in llvm#145878. A dependency was added in
the wrong direction, causing an assertion failure due to broken
topological order.

(cherry picked from commit 6df012a)
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
This patch fixes a bug introduced in llvm#145878. A dependency was added in
the wrong direction, causing an assertion failure due to broken
topological order.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants